Hey @kvnhmn 🙌

This is amazing, thank you for taking a run at this.
We would love to help to get this over the line.
The bulk of the PR looks great 🙇

One thing that would help us would be to leverage the protobuf struct type (instead of the string type).
Really, we would like our old attachment to leverage that too, but its tricky with backwards compat and protobuf.
I added a comment in the review that gives some examples of where we use it in the auth storage layer for metadata there.

Another thing that would help is to update the declarative format.
You can find the definition for Flag here:

Having Metadata here will allow folks to write it in yaml when either using a declarative backend or importing/exporting via the API.
This will need updating also in import and export:

• import

  flipt/internal/ext/importer.go

  Lines 123 to 129 in fee220d

  	req := &flipt.CreateFlagRequest{
  	Key: f.Key,
  	Name: f.Name,
  	Description: f.Description,
  	Enabled: f.Enabled,
  	NamespaceKey: namespace,
  	}

• export

  flipt/internal/ext/exporter.go

  Lines 129 to 135 in fee220d

  	flag := &Flag{
  	Key: f.Key,
  	Name: f.Name,
  	Type: f.Type.String(),
  	Description: f.Description,
  	Enabled: f.Enabled,
  	}

Lastly, would be awesome to get at-least a happy path case in the integration tests.
But also, don't want to overload you. This could also be something we do in a subsequent PR.

Thanks for contributing 🙌

Thanks for making these changes @kvnhmn . It is looking great.

Unsure if you can see the failures in CI, but thought I would shine a light on some of them.

One of our test suites is seeing a non-nil pointer to an empty metadata:

         	            	--- Expected
        	            	+++ Actual
        	            	@@ -16779,3 +16779,16 @@
        	            	  Type: (flipt.FlagType) 0,
        	            	- Metadata: (*structpb.Struct)(<nil>)
        	            	+ Metadata: (*structpb.Struct)({
        	            	+  state: (impl.MessageState) {
        	            	+   NoUnkeyedLiterals: (pragma.NoUnkeyedLiterals) {
        	            	+   },
        	            	+   DoNotCompare: (pragma.DoNotCompare) {
        	            	+   },
        	            	+   DoNotCopy: (pragma.DoNotCopy) {
        	            	+   },
        	            	+   atomicMessageInfo: (*impl.MessageInfo)(<nil>)
        	            	+  },
        	            	+  sizeCache: (int32) 0,
        	            	+  unknownFields: ([]uint8) <nil>,
        	            	+  Fields: (map[string]*structpb.Value) <nil>
        	            	+ })
        	            	 })

Which, I think is fine for the API to return this, but either the API needs to change or that test case needs to be updated to ensure it is asserting a non-nil empty struct.

The other thing I am noticing is that the AFTER keyword on ALTER appears to not be supported by all of the relational DB backends.
What is weird is that the SQLite docs says it does not support it, but also, it doesn't fail on migration. Whereas e.g. Postgres fails explicitly during the migrations:

 35  : [27.4s] | Error: running migrations: migration failed: syntax error at or near "AFTER" (column 44) in line 1: ALTER TABLE flags ADD COLUMN metadata JSON AFTER enabled;
35  : [27.4s] |  (details: ERROR: syntax error at or near "AFTER" (SQLSTATE 42601))

We might need to drop that from the relevant migrations. It seems like maybe MySQL supports it, but not postgres and potentially SQLite just ignores it (so no harm I guess).

looking great! commented on failing ci tests

Looks great! Thank you both @kvnhmn and @erka !